Skip to content

fix: skip permissions missing from destination during role sync#503

Closed
nathantournant wants to merge 3 commits intomainfrom
nathan.tournant/fix-role-permission-remap
Closed

fix: skip permissions missing from destination during role sync#503
nathantournant wants to merge 3 commits intomainfrom
nathan.tournant/fix-role-permission-remap

Conversation

@nathantournant
Copy link
Copy Markdown
Member

@nathantournant nathantournant commented Apr 1, 2026

Summary

Two fixes for cross-DC integration test stability:

  • roles: remap_permissions now drops permissions that exist in the source but not the destination (e.g. bits_dev_write behind a feature flag), instead of leaving raw name strings that cause 400 Bad Request: invalid UUID
  • notebooks: Added attributes.tags to excluded_attributes — the Notebooks API injects human_edited:true on update, causing phantom diffs that never converge

Root cause

  • The source org (EU1) has beta-permission-bits_dev_write feature flag enabled; the destination org (US5) does not. Standard roles include this permission, and cross-DC sync failed on every role operation.
  • The Notebooks API auto-tags edited notebooks with human_edited:true, a server-managed tag not present in source resource files.

Test plan

  • All 211 unit tests pass
  • Integration tests: permission fix resolved all cleanup teardown failures (26→2), notebook fix should resolve the remaining 2

🤖 Generated with Claude Code

…_update_sync

In test_resource_update_sync, the --filter argument was appended to
diff_cmd instead of sync_cmd due to a copy-paste error. This caused
the sync step to run unfiltered in live integration tests (RECORD=none),
potentially syncing resources outside the test scope. Cassette-based
tests masked the issue since VCR replays fixed responses regardless.

Adds regression tests validating that command-building logic places
filter arguments in the correct command lists.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nathantournant nathantournant requested a review from a team as a code owner April 1, 2026 08:24
@nathantournant nathantournant force-pushed the nathan.tournant/fix-role-permission-remap branch from 5869c12 to 417bf57 Compare April 1, 2026 08:32
@nathantournant nathantournant changed the base branch from fix/test-update-sync-filter-cmd to main April 1, 2026 08:37
@nathantournant nathantournant force-pushed the nathan.tournant/fix-role-permission-remap branch 2 times, most recently from a6a9f99 to b9ea16b Compare April 1, 2026 11:30
@nathantournant nathantournant changed the base branch from main to fix/test-update-sync-filter-cmd April 1, 2026 11:30
@nathantournant nathantournant changed the base branch from fix/test-update-sync-filter-cmd to main April 1, 2026 11:57
@nathantournant nathantournant force-pushed the nathan.tournant/fix-role-permission-remap branch from 8cc4b00 to 39c74b5 Compare April 1, 2026 12:14
When syncing roles cross-DC, permissions gated behind feature flags
(e.g. bits_dev_write) may exist in the source org but not the
destination. Previously, remap_permissions left the unresolved
permission name as a raw string in the payload, causing the Roles API
to reject it with "invalid UUID [permission_name]".

Now, permissions that exist in the source but have no match in the
destination are dropped from the role payload with a warning log.
This prevents a single missing permission from breaking the entire
role sync — and by extension every integration test teardown.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nathantournant nathantournant force-pushed the nathan.tournant/fix-role-permission-remap branch from 39c74b5 to 3a878c7 Compare April 1, 2026 13:01
The Notebooks API now injects ai_generated, ai_edited, and human_edited
tags on every write based on request characteristics. Since the sync CLI
uses API key auth (not MCP), every PUT is classified as a human edit,
causing human_edited:true to be added server-side. This creates a
non-converging diff because the source notebook lacks the tag while the
destination always gets it re-injected.

Strip these server-managed tags in handle_special_case_attr so they are
ignored on both sides during import, sync, and diff. Legitimate user
tags (team:*, llm-observability:*) are unaffected.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nathantournant nathantournant force-pushed the nathan.tournant/fix-role-permission-remap branch from 8037480 to a6b4c19 Compare April 1, 2026 15:05
@DataDog DataDog deleted a comment from michael-richey Apr 1, 2026
@nathantournant
Copy link
Copy Markdown
Member Author

Changes ported to #502

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant